Report review-required policy refusals before Self-merge - #708

Summary

before each Self-merge (Run or Spec PR)
  read reviewDecision for the captured PR
  REVIEW_REQUIRED / CHANGES_REQUESTED → review required policy refusal
  approved / empty / unknown / failed read → existing guarded merge

Review-required refusals exit 1, leave the PR ready with its Claim kept, and skip the merge attempt and Repair round. stderr, Run notifications and the Activity log name the reason.

Closes #707

Evidence

• Before: the initial focused CLI regression failed: REVIEW_REQUIRED still merged the PR and exited 0 instead of 1.
  After: cargo test --test merge_run a_required_review_refuses_the_merge_without_another_round_and_names_the_outcome -- --exact exercises the consolidated regression for both blocking decisions: no merge/Repair, PR stays ready, Claim kept, cleanup completes. Coverage includes another read after Repair, unknown/null decisions, and failed reads with successful merges or other policy refusals.
• Before: cargo test --test run_notification a_review_required_notification_and_summary_name_the_refusal -- --exact failed with the subject ending : failed.
  After: it passes with review required in the notification and Activity log. The Spec PR test also passes for both blocking decisions.
• Validation: cargo check --all-targets, cargo fmt --check, and cargo clippy --all-targets -- -D warnings pass. cargo test -- --test-threads=4 passes for the full Rust suite.

Merge Danger

Door: two-way

Reversible code and documentation change; no migrations.

Blast Radius: Self-merge

Adds one best-effort GitHub read immediately before each merge request. Other policy refusals retain their existing handling.

Test seams

• The thirdshift CLI against real Git and fake gh: Self-merge decisions, Repair, PR readiness, Claim retention, cleanup, stderr/stdout and exit status, for Run and Spec run.
• Run notification delivery through the local Resend stand-in and the Activity log summary.

Unaddressed findings

Standards

• STD-001 / S1 (same optional Low finding; declined): Retain the existing string-based reporting interface (src/run_ending.rs:94) for this scoped outcome addition. This is an implementation plan decision: Account already carries the display outcome; summary reuses it at lines 279–281, while the policy decision itself remains typed as PolicyRefusal::ReviewRequired at lines 295–299. Adding a second reporting category or stored summary prefix solely for this label would duplicate that state. The reviewer explicitly identified a maintenance judgment, not incorrect behavior. No Spec or Day shift decision is needed.

Spec

None.

Security

None. Single-session guidance review of all 13 changed files and supporting PR identity, guarded merge, interruption, failure cleanup, and notification code against pinned merge base 3574ae7267507e8fd67403538e0a851d8030f1d6 found no introduced security findings. Read the relevant attack-class guidance; no repository SECURITY.md or threat model was present.

Validation: 29 review-focused tests across merge_run, spec_run, and run_notification passed in a network-isolated sandbox using local stand-ins. No source fixes were needed.

Review coverage

Reviewed against main with fresh, independent Standards and Spec reviewers after reconciling the branch. The final pass reported no findings; the initial pass's optional STD-001 remains documented above. Both read all 13 changed files; unread changed files: none. Reports are retained in .thirdshift-review-1tPh0F/standards.md and spec.md in the Run worktree.

Built with codex · gpt-6.1-sol · high